Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gazebo support for KUKA KR10 #125

Open
wants to merge 9 commits into
base: melodic-devel
Choose a base branch
from

Conversation

cschindlbeck
Copy link

Experimental support for Gazebo KUKA KR10.

Currently on kinetic and i have my custom colors applied

@cschindlbeck
Copy link
Author

cschindlbeck commented Apr 17, 2018

Reverted changes to adhere to indigo convention, changed to default KUKA colors, and added dummy inertial values

@gavanderhoorn
Copy link
Member

I realise that getting anything but approximates is difficult/impossible, but the current dummy values give all links a mass of 10 Kg. That seems a bit much.

There are a few ways to estimate masses and inertias from meshes, such as using Meshlab or using SolidWorks. Another alternative could be to use approximations based on primitive shapes (such as is done in the universal_robot urdfs (here)).

We could consider adding similar macros as the one in universal_robot to kuka_resources. That way we can reuse it in other support packages as well.

@gavanderhoorn
Copy link
Member

Related #128.

@cschindlbeck
Copy link
Author

cschindlbeck commented Apr 23, 2018

Another alternative could be to use approximations based on primitive shapes (such as is done in the universal_robot urdfs (here)).

The mass values of the UR look like accurate values (beside the first one). I can only make estimates on the mass. Using a macro for cylindrical inertia estimates seems like a good idea. I will try to implement it in the near future.

@cschindlbeck
Copy link
Author

So i added a common_robotparameters.xacro in kuka_resources where we could store all "identified" robot parameters such as mass, link lengths in DH notation, cog etc
I also added the cylindrical inertia that takes values from this file

What do you think?

@gavanderhoorn
Copy link
Member

Hi, thanks for the PR.

Adding the cylinder_inertial macro is a good thing, but I'm not so sure I like creating a single file with lots of parameters for "all" robots. Can you give a reason why those parameters could not just be part of the xacro macro itself?

@gavanderhoorn
Copy link
Member

I'm also not sure about using DH for this. Afaik, KUKA does not use DH, so it seems strange to start doing that in this repository.

@cschindlbeck
Copy link
Author

My ideas were:

  • The kuka agilus series has many common link lenghts, therefore using a single file would mitigate redundancy/error proneness
  • DH notation is a common convention that would help to standardize all ros_industrial supported robots

I will the put the parameters to the respective macro file and use the notation from the KUKA manual instead (i will do this in 2-3 weeks).

@gavanderhoorn
Copy link
Member

gavanderhoorn commented May 4, 2018

@cschindlbeck wrote:

My ideas were:

  • The kuka agilus series has many common link lenghts, therefore using a single file would mitigate redundancy/error proneness

Ok, that could make sense. This would connect to the discussion @BrettHemes and I had in #105.

re: reduce errors: we're still using text-based identifiers with no robust way of verifying that we've used the correct one other than looking at the model in RViz or manual inspection of the xacro macro text. It feels a bit like 'fixing' one side but introducing another possible source of errors.

  • DH notation is a common convention that would help to standardize all ros_industrial supported robots

Yes, it's a common convention, but as I wrote earlier, afaik KUKA doesn't use it. That means we'd have to rely on contributors to derive the DH parameters for us, which doesn't seem like it improves things (compared to how we do it now in the xacro macros).

I will the put the parameters to the respective macro file and use the notation from the KUKA manual instead (i will do this in 2-3 weeks).

No problem.

@cschindlbeck
Copy link
Author

I have put all code into the kr10r1100sixx_macro.xacro.
I still think it would make sense to outsource the cylinder_inertial macro but i leave this discussion to #105
Do the link names make sense? I haven't found a naming convention in the KUKA documentation that i could use.

@gavanderhoorn
Copy link
Member

@cschindlbeck wrote:

Do the link names make sense? I haven't found a naming convention in the KUKA documentation that i could use.

I'm not sure which link names you are referring to?

I still think it would make sense to outsource the cylinder_inertial macro but i leave this discussion to #105

And I completely agree :) see my earlier #125 (comment):

We could consider adding similar macros as the one in universal_robot to kuka_resources. That way we can reuse it in other support packages as well.

@cschindlbeck
Copy link
Author

I'm not sure which link names you are referring to?

These:

+  <!-- Kinematic parameters -->
+  <xacro:property name="kuka_kr10r1100sixx_d1" value="0.400" />
+  <xacro:property name="kuka_kr10r1100sixx_d4" value="0.515" />
+  <xacro:property name="kuka_kr10r1100sixx_a1" value="0.025" />
+  <xacro:property name="kuka_kr10r1100sixx_a2" value="0.560" />
+  <xacro:property name="kuka_kr10r1100sixx_a3" value="0.035" />
+  <xacro:property name="kuka_kr10r1100sixx_a5" value="0.080" />

as in here

@gavanderhoorn
Copy link
Member

gavanderhoorn commented May 23, 2018

Ah.

I'm not too hung up on particular names in this case.

It might make sense to move those property defs inside the kuka_kr10r1100sixx macro definition though. If I'm not mistaken that would make them local to the macro, removing the need to prefix them with kuka_kr10r1100sixx_ to give them a namespace.

Jade+ xacro should support local properties like that, see wiki/xacro: Local properties:

Properties and macros defined within a macro are local to this macro, i.e. not visible outside.

It would be good to give the properties a bit more of a descriptive name though, I'm not really a fan of single-character variable names. I understand these come from DH, but since we're not using that (and consequently our frame orientations don't align with it) I'm not sure using DH names for these lengths still makes sense?

@cschindlbeck
Copy link
Author

I shortened the names and made them more descriptive.

@simonschmeisser simonschmeisser changed the base branch from indigo-devel to melodic-devel September 23, 2021 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants